-
Notifications
You must be signed in to change notification settings - Fork 133
Fix: Camera is not muted when the earpiece mode is enabled #3596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: livekit
Are you sure you want to change the base?
Conversation
| * Used for video to stop camera when earpiece mode is on. | ||
| * @private | ||
| */ | ||
| private readonly forceMute$: Observable<boolean>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is also what we should use if we run have an error for any of the devices. Add the force mute flag and then render the button as in disabled state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we have to check for these errors and when they occur. But maybe now it will just make the available map empty, that is already muting. What error are you thinking about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An error while muting/unmuting. But I think I mixed things up since if we use the lower level api mute unmute this will never throw.
| this.handler$.next(defaultHandler); | ||
| } | ||
|
|
||
| private readonly devicesConnected$ = combineLatest([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I like this name. devicesConnected now is implied by force mute?
I would expect the other way around, that no connected device implies force muting.
But force mute = true -> no devices connected feels like it messes with causality.
| * Used for video to stop camera when earpiece mode is on. | ||
| * @private | ||
| */ | ||
| private readonly forceMute$: Observable<boolean>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An error while muting/unmuting. But I think I mixed things up since if we use the lower level api mute unmute this will never throw.
See #3587
Looks like there was a merge problem? These changes have been lost #3351
I integrated it a bit differently and added the test.
We should get rid of MuteStates it is way too complex to understand, and since the refactoring it looks like part of what it is doing should be under the responsability of the Publisher/LocalMembership